-
-
Notifications
You must be signed in to change notification settings - Fork 311
Semantic versioning update #5944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
versioning with link to semantic versioning wiki page. Update H5.c and version tests for move of major and minor versions to 1st and 2nd version numbers.
…tic-versioning-update
…x/hdf5 into semantic-versioning-update
test/CMakeTests.cmake
Outdated
| if ("H5TEST-tcheck_version-major" MATCHES "${HDF5_DISABLE_TESTS_REGEX}") | ||
| set_tests_properties (H5TEST-tcheck_version-major PROPERTIES DISABLED true) | ||
| endif () | ||
| # minor + 1 should pass on non-develop branches |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But minor test KEEPS: WILL_FAIL "true"
Release test REMOVES: WILL_FAIL "true"
so
# minor + 1 should FAIL on develop branch (minor=0 in exceptions)
# minor + 1 should PASS on release branches (minor!=0 not in exceptions)
# This test expects failure when run on develop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't think we can make AI follow this, so we propose to put 999 in the VERS_MINOR_EXCEPTIONS list that will trigger the warning and abort for incompatible minor versions, and test 999 in the tcheck_versions -tm test. Any future incompatible minor versions can be added to that list (there should never be any). The release branches can run the same test, and because WILL_FAIL is set to true, the test will pass on both develop and the release branches. If a truly incompatible branch is added to the list of exceptions and using the incompatible branch is attempted the warning and abort will be triggered as WILL_FAIL is applied only to the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, are you working on this? I don't see the changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments
(1) major must match
(2) minor version backward compatibility check
if minor versions don't match
2a, check if either version is in the incompatible list (e.g., develop)
print incompatibility warning and HDF5_DISABLE_VERSION_CHECK options
abort if version check not disabled
2b, check if header minor version is newer than library version
print forward compatibility warning and HDF5_DISABLE_VERSION_CHECK options
abort if version check not disabled
(3) release (patch) versionnot checked
(4) VERS_MINOR_EXCEPTIONS list now set to '999', and minor version '999' checked in
test/tcheck_version.c. This check will always cause the -tm option to fail and
can remain in release branches, eliminating need to change or remove WILL_FAIL true
for release branches.
|
Current status: (1) major must match (3) release (patch) version not checked |
branches - removed that instruction fromRELEASE_PROCESS. Change release version instructions to use x.y.z.1 for pre-release instead of x.y.z-1 as for develop snapshots.
…x/hdf5 into semantic-versioning-update
|
Release process document refers this link but I don't see it updated for v200 yet as of 2025-10-30. [u13] https://support.hdfgroup.org/documentation/hdf5/latest/api-compat-macros.html Will it be updated after the release automatically? |
@byrnHDF, I think this is a page created with doxygen? I'm thinking about adding a RELEASE_PROCESS.md entry to update it if needed, but it should probably be done before that. Maybe the RELEASE_PROCESS.md task should be to check that it's updated? Also, this is one of 25 technical notes on https://support.hdfgroup.org/documentation/hdf5/latest/_t_n.html. I wonder if any of the others might go out of date. I see one named HDF5 Library Release Version Numbers that I intend to check today. |
…cumentation is updated for the version to be released.
…x/hdf5 into semantic-versioning-update
|
|
||
| } /* end if (H5_VERS_RELEASE != relnum) */ | ||
| /* Check for forward compatibility usage. */ | ||
| if (H5_VERS_MINOR > minnum) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic is backwards, Library minor > Header minor
Example: Library 2.1.x, Headers 2.0.x
Meaning: App compiled with OLD headers, linked with NEW library
This is backward compatible, so it should be allowed, but it aborts.
It should be < minnum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this block is for Header minor > library minor, to warn user that for example app compiled with 2.1 headers is linking to 2.0 library. A message for this particular situation is added in lines 840 - 848.
Allen's arguments were that there should be a warning for [accidentally] linking to an older minor version library, but that it could still work as long as functions not in the older library were not used in the App.
There is no failure for backward compatibility unless one of the two minor versions is in the list of exceptions to minor version compatibility, which basically should not happen. Otherwise, if the Header Minor version < library minor version it will always be compatible and there is no warning, and no abort.
There is this comment (from before, just with 'release' changed to 'minor' in line 917 that I question now: "/* Library develop minor versions are incompatible by design */"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify this, and is this information correct?
H5_VERS_MINOR = The minor version number compiled into the library
minnum = The minor version number from the headers the app was compiled with
"Header minor > library minor, to warn user that app compiled with 2.1 headers is linking to 2.0 library"
App compiled with 2.1 headers, minnum = 1
Linked with 2.0 library, H5_VERS_MINOR = 0
if (H5_VERS_MINOR > minnum) , if (0 > 1) , FALSE
Does not go into the warning block at all.
Instead it warns in the opposite case. Is the intent to warn with headers > library (forward incompat)
Important
Update semantic versioning process and adjust version checking logic for minor version compatibility in HDF5.
RELEASE_PROCESS.md.VERS_RELEASE_EXCEPTIONStoVERS_MINOR_EXCEPTIONSinH5.c.H5_check_version()to focus on minor version compatibility instead of release version.WILL_FAIL "true"for minor version check inCMakeTests.cmaketo allow minor version +1 to pass on non-develop branches.This description was created by
for de3e556. You can customize this summary. It will automatically update as commits are pushed.